Skip to content

Correct key name in arduino-cli compile --dump-profile output #1846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 24, 2022
Merged

Correct key name in arduino-cli compile --dump-profile output #1846

merged 1 commit into from
Aug 24, 2022

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Aug 23, 2022

The key name is hereby corrected.

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • [N/A] Docs have been added / updated (for bug fixes / features)
  • [N/A] UPGRADING.md has been updated with a migration guide (for breaking changes)

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

The --dump-profile flag causes Arduino CLI to print the board and sketch dependencies of that compilation as a YAML document. This could be copied into a "sketch project file" for later use as a "build profile" specified via the --profile flag.

A typo in the profiles key name caused the generated "build profile" entry to not be recognized when added to a "sketch project file":

arduino-cli compile --fqbn arduino:avr:uno --dump-profile
Sketch uses 3720 bytes (11%) of program storage space. Maximum is 32256 bytes.
Global variables use 393 bytes (19%) of dynamic memory, leaving 1655 bytes for local variables. Maximum is 2048 bytes.

profile:
  uno:
    fqbn: arduino:avr:uno
    platforms:
      - platform: arduino:avr (1.8.5)

Used library   Version Path
EEPROM         2.0     C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.8.5\libraries\EEPROM
SoftwareSerial 1.0     C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.8.5\libraries\SoftwareSerial

Used platform Version Path
arduino:avr   1.8.5   C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.8.5

What is the new behavior?

The arduino-cli compile --dump-profile output uses the valid key name profiles.

arduino-cli compile --fqbn arduino:avr:uno --dump-profile
Sketch uses 3720 bytes (11%) of program storage space. Maximum is 32256 bytes.
Global variables use 393 bytes (19%) of dynamic memory, leaving 1655 bytes for local variables. Maximum is 2048 bytes.

profiles:
  uno:
    fqbn: arduino:avr:uno
    platforms:
      - platform: arduino:avr (1.8.5)

Used library   Version Path
EEPROM         2.0     C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.8.5\libraries\EEPROM
SoftwareSerial 1.0     C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.8.5\libraries\SoftwareSerial

Used platform Version Path
arduino:avr   1.8.5   C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.8.5

Does this PR introduce a breaking change, and is titled accordingly?

No breaking change.

Other information

Originally reported at https://forum.arduino.cc/t/dump-profile-error/1024698

This indicates to me that the existing dedicated infrastructure for printing build profiles should be used instead of duplicating what is already hacky code all over again specifically for use with the --dump-profile flag:

arduino-cli/arduino/sketch/profiles.go

Line 56 in f43c9ec

func (p *Profiles) UnmarshalYAML(unmarshal func(interface{}) error) error {

@per1234
Correct key name in arduino-cli compile --dump-profile output
740d0f7 
The `--dump-profile` flag causes Arduino CLI to print the board and sketch dependencies of that compilation as a YAML
document. This could be copied into a "sketch project file" for later use as a "build profile" specified via the
`--profile` flag.

A typo in the `profiles` key name caused the generated "build profile" entry to not be recognized when added to a
"sketch project file".

The key name is hereby corrected.
@per1234 per1234 added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project topic: CLI Related to the command line interface labels Aug 23, 2022
@per1234 per1234 requested a review from cmaglie August 23, 2022 05:37
@per1234 per1234 self-assigned this Aug 23, 2022
@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #1846 (740d0f7) into master (f43c9ec) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1846      +/-   ##
==========================================
- Coverage   36.33%   36.28%   -0.05%     
==========================================
  Files         232      232              
  Lines       19496    19496              
==========================================
- Hits         7083     7074       -9     
- Misses      11585    11592       +7     
- Partials      828      830       +2
Flag Coverage Δ
unit 36.28% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cli/compile/compile.go 0.00% <0.00%> (ø)
arduino/monitor/monitor.go 41.05% <0.00%> (-6.32%) ⬇️
arduino/cores/packagemanager/package_manager.go 63.02% <0.00%> (+0.84%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@cmaglie
Copy link
Member

cmaglie commented Aug 24, 2022

This indicates to me that the existing dedicated infrastructure for printing build profiles should be used instead of duplicating what is already hacky code all over again specifically for use with the --dump-profile flag:

Agreed, would you like to give it a try? I'll merge this PR in the meantime to fix the bug.

@cmaglie cmaglie merged commit 6138864 into arduino:master Aug 24, 2022
@per1234
Copy link
Contributor Author

per1234 commented Aug 24, 2022

would you like to give it a try?

Yes. I had already jotted it down on my "to do" list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmaglie cmaglie cmaglie approved these changes

@MatteoPologruto MatteoPologruto Awaiting requested review from MatteoPologruto

@ubidefeo ubidefeo Awaiting requested review from ubidefeo

Assignees

@per1234 per1234

Labels
topic: CLI Related to the command line interface topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@per1234 @cmaglie